-
Notifications
You must be signed in to change notification settings - Fork 370
feat(CC-batch-7): added batch 7 #11869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Preview: https://patternfly-react-pr-11869.surge.sh A11y report: https://patternfly-react-pr-11869-a11y.surge.sh Preview: https://pf-react-pr-11869.surge.sh A11y report: https://pf-react-pr-11869-a11y.surge.sh |
1bbee4a
to
6ccfde1
Compare
fd4bf49
to
f1546dc
Compare
f1546dc
to
540457e
Compare
{props.notificationDrawerHeader} | ||
<NotificationDrawerBody> | ||
{props.notificationDrawerGroup} | ||
<NotificationDrawerList>{props.notificationDrawerNotifications}</NotificationDrawerList> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this example have both optional groups AND a list? I think you'd want either/or
onClose={props.hasActionsMenu.onClose} | ||
title={props.headingText} | ||
> | ||
{props.hasActionsMenu.dropdown} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this still work if the designer doesn't pass a dropdown? Or does figma automatically always have a dropdown? It's not a required prop.
packages/code-connect/components/NotificationDrawer/NotificationDrawerItem.figma.tsx
Outdated
Show resolved
Hide resolved
// enum | ||
type: figma.enum('Type', { | ||
Square: 'square', | ||
Circle: 'circle' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a default shape? in the PF examples, the default skeleton shape is a rectangle if square or circle are not specified.
540457e
to
f87a7ce
Compare
}), | ||
showUnreadCount: figma.boolean('Has count', { | ||
true: 3, | ||
false: NaN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
count
is required to NaN
may cause issues, maybe set to 0
instead.
{ | ||
props: { | ||
// boolean | ||
hasCount: figma.boolean('Has count', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused, seems to be a dupe of showUnreadCount
.
isOpen={false} | ||
onOpenChange={() => {}} | ||
popperProps={{ position: 'right' }} | ||
toggle={() => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pass the toggleRef
prop to MenuToggle
's ref
.
Success: 'success', | ||
Warning: 'warning', | ||
Danger: 'danger' | ||
}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is an additional custom
value allowed for variant
. I don't know if Figma
has it explicitly though. It's the default value of variant
if nothing is passed in.
// nested | ||
pageQuantity: figma.nestedProps('Page quantity selector', { | ||
itemCount: figma.string('Total quantity'), | ||
state: figma.enum('State', { Disabled: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The state
prop looks unused.
'Bottom-middle': 'bottom', | ||
'Bottom-right': 'bottom-end' | ||
}), | ||
status: figma.enum('Status', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
icon
is unused in the code connect
component={props.isLink.component} | ||
href={props.isLink.href} | ||
isActive={props.isActive} | ||
key="simple-list-item-key" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
key
is probably unneeded here. It's not a prop of SimpleList, and was probably only in the example because the list items were being created in a array.
'4XL': '4xl' | ||
}) | ||
}, | ||
example: (props) => <Skeleton fontSize={props.size} type={props.type} screenreaderText="Loading default content" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no type
prop, this should be shape
instead (which can be circle
or square
)
f87a7ce
to
46a6228
Compare
46a6228
to
3e17d45
Compare
Relates to: #11624
Included components:
Component tracker
Figma preview
Resources: